Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support to Golang codegen for booleans #4231

Merged
merged 1 commit into from
Apr 18, 2023

Conversation

mdehoog
Copy link
Contributor

@mdehoog mdehoog commented Apr 16, 2023

Currently Golang codegen passes through boolean types without type conversion. This generates invalid Golang, as boolean is not Golang keyword.

This PR copies Swift's implementation of a type map:

SwiftTypeMap ::= [
"int":"Int",
"float":"Float",
"long":"Int64",
"double":"Double",
"bool":"Bool",
"boolean":"Bool",
default : key
]

@mdehoog mdehoog marked this pull request as draft April 16, 2023 18:52
Signed-off-by: Michael de Hoog <michael.dehoog@coinbase.com>
@jimidle
Copy link
Collaborator

jimidle commented Apr 17, 2023

Where is that input coming from? Something in your grammar itself? Can you give me an example? I am not saying this isn't a good idea, but I have forgotten where the type map comes in to it to be honest?

Also, is there a reason for you to do force pushes? Just forgot the -s on the commit?

@mdehoog
Copy link
Contributor Author

mdehoog commented Apr 17, 2023

Here's some examples: https://github.com/ethereum/solidity/blob/c6aab84a0c25a9348e22326f12c2ebbf407989dc/docs/grammar/SolidityParser.g4#L161-L164 (see the PR linked to this one for context). Also related: ethereum/solidity#9679, which is relevant to the C++ codegen.

Reason for force-pushing is to keep the commit history clean (there was a small typo in the original commit i pushed that the CI flagged) (and yes, didn't realize the -s was a requirement for this repo).

@jimidle
Copy link
Collaborator

jimidle commented Apr 18, 2023

Here's some examples: https://github.com/ethereum/solidity/blob/c6aab84a0c25a9348e22326f12c2ebbf407989dc/docs/grammar/SolidityParser.g4#L161-L164 (see the PR linked to this one for context). Also related: ethereum/solidity#9679, which is relevant to the C++ codegen.

Reason for force-pushing is to keep the commit history clean (there was a small typo in the original commit i pushed that the CI flagged) (and yes, didn't realize the -s was a requirement for this repo).

Ah, I see. I thought it was about embedded code in the parser. On the face of it, this does not seem unreasonable. Let me check your changes...

@jimidle
Copy link
Collaborator

jimidle commented Apr 18, 2023

@parrt This can be merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants